Removal of time interval and closing files within BasisWriter::writeBasis #261
Conversation
dreamer2368
changed the title
dreamer2368
requested review from
chldkdtn,
ckendrick,
dylan-copeland and
siuwuncheung
|
@dylan-copeland , Even though the multiple time intervals are never used in practice, |
|
@dylan-copeland , per our discussion, I added a commit that spits an error message whenever a user attempts to increase time interval, directly or indirectly through Now I thought about the data format of basis classes, and ended up keeping the current implementation, allowing both csv and hdf5 format. Currently both data formats are compatible, and no reason to remove a functionality that we can support without any complication. With that, I think this PR is ready for review and merge. |
|
In my work for PR #274 , I ended up removing the concept of time interval. I moved those changes to here since it fits more to this PR. While the inside of all functions do not use the concept of time intervals, some function signatures remain in order to keep the backward compatibility:
Not exactly sure if we'd want to change these function signatures right away, which will impact all applications using libROM. Need more discussion about this @chldkdtn @ckendrick |
dreamer2368
changed the title
dreamer2368
force-pushed
the
generator-fix2
branch
from
e16e7ba to
3d957bc
Compare
dreamer2368
force-pushed
the
generator-fix2
branch
from
302f190 to
30ff855
Compare
|
Dataset names for basis are now updated, removing the trailing 6 digits of integer that used to indicate time interval. Any out-dated snapshot/basis hdf5 files can be updated using This basis/snapshot format update is tested with |
|
@ebchin , After some discussion throughout this PR, we decided to change the dataset names within snapshot/basis hdf5 files. I heard from @dylan-copeland that you might have data files for your production and your simulation can be impacted by this update. I prepared a python script |
…asis (#261) * BasisWriter::writeBasis create/open a file and closes the file at the end. * stylization. * HDFDatabase::putIntegerArray - overwrites if the dataset exists. * enforce single time interval in Options. * HDFDatabase::putIntegerArray does not allow overwrite. * BasisWriter::writeBasis always create the file, which will overwrite the exisiting file. * add a header and stylization. * remove increase time interval test, as time interval is fixed to 1. * add an error message for a guidance. * remove test_SVD from ci workflow. * SVD::increaseTimeInterval - allow the initial time interval. * minor fix in test_IncrementalSVDBrand. * reflecting the comments. * removed the concept of time interval in BasisReader. time argument remains for backward compatibility. * BasisWriter: removed the concept of time interval. * minor fix in BasisReader. * SVD: removed the concept of time intervals. * BasisGenerator: removed the concept of time interval. * add test_SVD.cpp for resolving conflict. * stylization. * changed function signature of BasisGenerator::takeSample. * rebased to resolve conflict. * changed function signature for BasisReader::getSpatialBasis. * changed function signature for BasisReader::getTemporalBasis. * changed function signature for BasisReader::getSingularValues. * changed function signature for BasisReader::getSnapshotMatrix. * removed Options::max_time_intervals * SVD::isNewSample -> isFirstSample * update comments for StaticSVD::isBasisCurrent * minor comment updates. * stylization. * update dataset names and added python script for dataset name update. * fixed the ci workflow * minor fix in ci workflow. * minor fix * removed BasisReader:isNewBasis * minor doxygen comment update
Closing files at the end of
BasisWriter::writeBasisThis PR revamps #150 . Now closing the database file is not associated with the destruction of
BasisWriter. This enables writing both the basis and the snapshot at a given moment. As far as I believe, this should keep the backward compatibility as well, but it needs other people's review as we do not have a regression test to check this.Databaseclass prints out the message when creating/opening a file.BasisWriterclass initializes/destroys the memberDatabase* d_databaseandDatabase* d_snap_databaseat its initialization/destruction, respectively. This, however, does not involve a file creation/opening/close.putIntegeroperation in~BasisWriteris moved intoBasisWriter::writeBasis.BasisWriter::writeBasisnow closes the file at the end of the function call.Removal of time interval
The concept of time interval is obsolete, and it hasn't been used in practice. This is removed, addressing #267 .
While the inside of all functions do not use the concept of time intervals, some function signatures remain in order to keep the backward compatibility:
BasisGenerator::takeSampleuseddouble timeonly for determining the time interval. Nowtimeis a dummy variable and not used in the function.BasisReaderuseddouble timefor determining the time interval, but nowtimeis a dummy variable.getSpatialBasisgetTemporalBasisgetSingularValuesgetSnapshotMatrixThe concept of time interval is obsolete, and it hasn't been used in practice. This will be removed in future (#267 ), and for now we enforce the number of interval not to be more than 1.Optionsclass will return error ifmax_time_intervals > 1. This will be kept until we actually remove the time interval concept.BasisWriter::writeBasisassumes to newly create the file every time it is called.HDF5Database::putIntegerArraydoes not overwrite the dataset. If the datasetnum_time_intervalsis attempted to be re-written again, then an error will be returned.